Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Run Prometheus on a different port, optionally. #3274

Merged
merged 15 commits into from
May 31, 2018
Merged

Conversation

hawkowl
Copy link
Contributor

@hawkowl hawkowl commented May 23, 2018

This should allow us to be resistant to Twisted melting down meaning we don't get metrics, in theory.

@hawkowl hawkowl requested review from erikjohnston and removed request for erikjohnston May 23, 2018 18:57
@hawkowl
Copy link
Contributor Author

hawkowl commented May 23, 2018

@erikjohnston this is based off the other PR, so will become much smaller when that one merges :)

@erikjohnston
Copy link
Member

I think my preference would be to make it configurable via the listeners config rather than reusing the old metrics_port config options. Maybe something like:

listeners:
- port: 9000
  type: metrics

Also, we need to update each component rather than just the main one, i.e. everything else in synapse/app/* (yes, the duplication there makes me sad too)

@erikjohnston erikjohnston assigned hawkowl and unassigned erikjohnston May 24, 2018
@turt2live
Copy link
Member

If the configuration changes, please yell loudly in the changelog too 😇

@erikjohnston
Copy link
Member

@turt2live I imagine we'd still support hosting the metrics endpoint in the twisted reactor as well, so everything should be backwards compat :)

@hawkowl hawkowl assigned erikjohnston and unassigned hawkowl May 28, 2018
@hawkowl
Copy link
Contributor Author

hawkowl commented May 28, 2018

@erikjohnston New docs, added the config to all the apps, listeners config added.

Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo, looks good! Just a couple of comments.


for host in bind_addresses:
reactor.callInThread(start_http_server, int(port),
addr=host, registry=RegistryProxy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start_http_server should spawn its own thread, so is it necessary to callInThread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had issues with it jamming Twisted, for some reason, and this made it work fine. the mysteries of threading

"compress": False,
},
]
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not need to keep this for backwards compat for people using metrics_port?

@erikjohnston erikjohnston assigned hawkowl and unassigned erikjohnston May 29, 2018
@hawkowl hawkowl assigned erikjohnston and unassigned hawkowl May 31, 2018
@hawkowl
Copy link
Contributor Author

hawkowl commented May 31, 2018

@erikjohnston I put back the old metrics_port, and raised a warning saying it's deprecated, with a link to the new docs. I couldn't find any better globally accessible link than just the file in the repo, though...

@hawkowl hawkowl merged commit febe0ec into develop May 31, 2018
@hawkowl hawkowl deleted the hawkowl/prom-diff-port branch May 31, 2018 09:05
- type: metrics
port: 9000
bind_addresses:
- '0.0.0.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this just IPv4 rather than an IPv6 address too? (I see f551ac3 changed this, but the commit comment is, ahem, unenlightening.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SocketServer (from the stdlib that Prometheus Client uses) only supports IPv4 out of the box, so it refuses IPv6 addresses. :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whaaa


All time duration-based metrics have been changed to be seconds. This affects:

================================
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neilisfragile added a commit that referenced this pull request Jun 6, 2018
Changes in synapse v0.31.0 (2018-06-06)
======================================

Most notable change from v0.30.0 is to switch to python prometheus library to improve system
stats reporting. WARNING this changes a number of prometheus metrics in a
backwards-incompatible manner. For more details, see
`docs/metrics-howto.rst <docs/metrics-howto.rst#removal-of-deprecated-metrics--time-based-counters-becoming-histograms-in-0310>`_.

Bug Fixes:

* Fix metric documentation tables (PR #3341)
* Fix LaterGuage error handling (694968f)
* Fix replication metrics (b7e7fd2)

Changes in synapse v0.31.0-rc1 (2018-06-04)
==========================================

Features:

* Switch to the Python Prometheus library (PR #3256, #3274)
* Let users leave the server notice room after joining (PR #3287)

Changes:

* daily user type phone home stats (PR #3264)
* Use iter* methods for _filter_events_for_server (PR #3267)
* Docs on consent bits (PR #3268)
* Remove users from user directory on deactivate (PR #3277)
* Avoid sending consent notice to guest users (PR #3288)
* disable CPUMetrics if no /proc/self/stat (PR #3299)
* Add local and loopback IPv6 addresses to url_preview_ip_range_blacklist (PR #3312) Thanks to @thegcat!
* Consistently use six's iteritems and wrap lazy keys/values in list() if they're not meant to be lazy (PR #3307)
* Add private IPv6 addresses to example config for url preview blacklist (PR #3317) Thanks to @thegcat!
* Reduce stuck read-receipts: ignore depth when updating (PR #3318)
* Put python's logs into Trial when running unit tests (PR #3319)

Changes, python 3 migration:

* Replace some more comparisons with six (PR #3243) Thanks to @NotAFile!
* replace some iteritems with six (PR #3244) Thanks to @NotAFile!
* Add batch_iter to utils (PR #3245) Thanks to @NotAFile!
* use repr, not str (PR #3246) Thanks to @NotAFile!
* Misc Python3 fixes (PR #3247) Thanks to @NotAFile!
* Py3 storage/_base.py (PR #3278) Thanks to @NotAFile!
* more six iteritems (PR #3279) Thanks to @NotAFile!
* More Misc. py3 fixes (PR #3280) Thanks to @NotAFile!
* remaining isintance fixes (PR #3281) Thanks to @NotAFile!
* py3-ize state.py (PR #3283) Thanks to @NotAFile!
* extend tox testing for py3 to avoid regressions (PR #3302) Thanks to @krombel!
* use memoryview in py3 (PR #3303) Thanks to @NotAFile!

Bugs:

* Fix federation backfill bugs (PR #3261)
* federation: fix LaterGauge usage (PR #3328) Thanks to @intelfx!
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants